Repository navigation
Fix swapped Hi L1B hae_latitude/hae_longitude and return longitude in [0, 360) from instrument_pointing - #3604
Merged
tmplummer merged 3 commits intoNov 4, 2026
Conversation
instrument_pointing returns (longitude, latitude), but compute_hae_coordinates assigned column 0 to latitude. Also wrap longitude into [0, 360) to match the CDF attributes and L1C PSET convention. Closes IMAP-Science-Operations-Center#3603 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the per-vector spiceypy.reclat loop with cartesian_to_spherical so (lon, lat) output uses the same [0, 360) longitude convention as IDEX and the Hi L1C PSET. Drop the now-redundant wrap in Hi L1B and add a mocked frame_transform test with known cartesian vectors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tmplummer
changed the title
tmplummer
commented
Oct 28, 2026
| if isinstance(et, typing.Collection): | ||
| return np.rad2deg([spiceypy.reclat(vec)[1:] for vec in pointing]) | ||
| return np.rad2deg(spiceypy.reclat(pointing)[1:]) | ||
| return cartesian_to_spherical(pointing, degrees=True)[..., 1:3] |
Contributor
Author
There was a problem hiding this comment.
This is a change to instrument_pointing when cartesian=False. Searching the repo, the only usage with cartesian=False was in the hi_l1b.py module. Two other uses of instrument_pointing all pass cartesian=True:
Contributor
|
LGTM! |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The review identifies no active issues and documents targeted fixes with regression coverage for coordinate ordering, normalization, quadrants, poles, and vector shapes.
No open issues 🎉
What changed in this PR
This pull request fixes swapped Hi L1B HAE latitude/longitude assignments and standardizes non-Cartesian instrument_pointing() output to return longitude in [0, 360) with latitude in [-90, 90].
Changes:
- Uses
cartesian_to_spherical()for normalized longitude handling. - Corrects Hi L1B longitude and latitude assignments.
- Adds regression coverage for coordinate ordering, wrapping, quadrants, poles, and vector shapes.
| File | Description |
|---|---|
imap_processing/tests/spice/test_geometry.py |
Tests longitude/latitude values, [0, 360) normalization, vector shapes, quadrants, and poles. |
imap_processing/tests/hi/test_hi_l1b.py |
Uses distinct mocked coordinates to detect swapped assignments. |
imap_processing/spice/geometry.py |
Updates instrument_pointing() conversion and documents output ranges and shapes. |
imap_processing/hi/hi_l1b.py |
Assigns longitude and latitude from the correct pointing columns. |
Review effort: Lite
leowerneck
approved these changes
Nov 4, 2026
leowerneck
left a comment
leowerneck
left a comment
Contributor
There was a problem hiding this comment.
LGTM. I made a minor comment
| By default, the coordinates returned are (Longitude, Latitude) coordinates in | ||
| the reference frame `to_frame`. Cartesian coordinates can be returned if | ||
| desired by setting `cartesian=True`. | ||
| By default, the coordinates returned are (Longitude, Latitude) coordinates, in |
Contributor
There was a problem hiding this comment.
There's a redundant "coordinates" in the first line; could drop the second one.
tmplummer
commented
Nov 4, 2026
tmplummer
merged commit Nov 4, 2026
2bc1d49
into
IMAP-Science-Operations-Center:dev
22 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Summary
Overview
hae_latitudeandhae_longitudewere swapped becauseinstrument_pointing()returns(longitude, latitude).instrument_pointing()now returns longitude in [0, 360), matching IDEX and the L1C PSET. Hi L1B is the only production caller that uses the (lon, lat) output; IDEX and Lo usecartesian=True. I decided to change the output of this function based on research indicating that in the heliophysics community, it is an agreed upon standard to report ecliptic longitude in the range:[0, 360)File changes
imap_processing/spice/geometry.py:instrument_pointing()usescartesian_to_spherical()in place of the per-vectorspiceypy.reclatloop; docstring documents output ranges.imap_processing/hi/hi_l1b.py:compute_hae_coordinates()assigns column 0 to longitude and column 1 to latitude.imap_processing/tests/spice/test_geometry.py: newtest_instrument_pointing_lon_latmocksframe_transformwith known cartesian vectors (all quadrants, poles, single and multiple inputs) and checks exact lon/lat values and the [0, 360) range.imap_processing/tests/hi/test_hi_l1b.py:test_compute_hae_coordinatesuses distinct lat/lon values so a swap is caught.Testing
test_hi_l1b.pyandtest_geometry.py; 321 pass across Hi L1B, geometry, IDEX, and Lo L1B.